Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RE-786: Quick verbiage and formatting fix missing from from RE-750 #512

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

davelcpanelnet
Copy link
Contributor

Should only add another 30 seconds or so of runtime with the artifacts moving about the stages.

@troglodyne
Copy link
Contributor

Commit message does not explain why this greatly improves things. Is it because it changes the timing of things, etc.?

Some other questions too... Do artifacts clean themselves up after or is that a manual process? I may not be 'hip' enough to what the CI is doing here yet to competently review this

Copy link
Contributor

@troglodyne troglodyne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked with Lanning about the PR and what exactly using artifacts gets us here. Basically it sounds like the runners don't really understand what each other are doing (it's some kind of bank queue) other than "what step am I supposed to be on given what steps have already executed." As such you need to store the IP somewhere that's accessible to the other runners, as we're not guaranteed to pick up the other steps on the same runner PID. Artifacts solve that IPC problem even though that's not their primary purpose. There's probably a more clever way to do this, but good enough is, especially since the public knowing these ephemeral IPs behind devwall doesn't do an attacker much good even if they were to deduce the IP ranges we use internally from this data.

Uploading logs for failed tests as an artifact so they can be linked at the end of the run is probably a good idea as well, not sure if that should be a follow up PR or done prior to merge here.

Anyways, approved.

Copy link

@annes449 annes449 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA skipped.

@davelcpanelnet davelcpanelnet changed the title Switch OpenStack Workflow to Using Uploaded/Downloaded Artifacts Betw… RE-786: Switch OpenStack Workflow to Using Uploaded/Downloaded Artifacts Betw… Sep 17, 2024
@davelcpanelnet davelcpanelnet changed the title RE-786: Switch OpenStack Workflow to Using Uploaded/Downloaded Artifacts Betw… RE-786: Quick verbiage and formatting fix missing from from RE-750 Sep 18, 2024
@toddr toddr merged commit 1273e60 into main Sep 18, 2024
3 checks passed
@toddr toddr deleted the RE-786 branch September 18, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants